-
Notifications
You must be signed in to change notification settings - Fork 12
Conversation
Incorrect call updated in update block. Moved OCTAudioEngine to setup.
Also rename method.
This test is failing sometimes. Maybe there is something race condition and toxAV is getting freed too early. Could you check that please? If nothing comes to your head you can simply add logging in test, so next time we would see what's happening there.
|
Looking into it.. I believe it's from the testStartAndStopMethod. When you
|
refToSelf needs to be set to NULL after deallocating toxAV. mocked_tox_iterate checks for the presence of OCTToxAVTests by using the pointer from refToSelf.
Okay, I think I solved it now. Was much simpler than I originally thought. Or at least I hope. 😄 |
|
Hm, it seems that mannol has deleted his user from github 😮 |
Hmm that's not good.... hopefully there's some explanation for this. Right now no one on IRC knows.. |
08f8199
to
f89220b
Compare
Okay finally got it to pass again 😅 Mannol added the old group audio chats but it was causing problems with the build, so I had to revert it back for now. |
8d73035
to
c1003f4
Compare
@@ -258,6 +260,25 @@ - (void)removeChatWithAllMessages:(OCTChat *)chat | |||
}); | |||
} | |||
|
|||
- (void)removeAllCalls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better to rename method to something more explicit like convertAllCallsToMessages
Updated, I'm not satisfied with how I did the enums, so feel free to give me any better suggestions |
event = OCTMessageCallEventUnanswered; | ||
break; | ||
case OCTCallStatusActive: | ||
event = OCTMessageCallEventAnswered; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add break;
here. This may save us from error in future.
case OCTToxAVAudioBitRate16: | ||
newBitrate = OCTToxAVAudioBitRate8; | ||
break; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks okay, you can just change the ordering to be more consistent. From lowest to highest.
Thanks for the comments, just updated |
With your branches in Antidote-for-Tox project I can see all updates in my feed. So no need to comment after updating now. :-) |
Demo has been implemented. Feel free to test it out 😄
Thanks